Skip to content

fix(api): wrap batch_update_tags store calls in asyncio.to_thread (BUG-JD-10)#90

Merged
pcalnon merged 1 commit intomainfrom
fix/bug-jd-10-async-batch-update-tags
May 6, 2026
Merged

fix(api): wrap batch_update_tags store calls in asyncio.to_thread (BUG-JD-10)#90
pcalnon merged 1 commit intomainfrom
fix/bug-jd-10-async-batch-update-tags

Conversation

@pcalnon
Copy link
Copy Markdown
Owner

@pcalnon pcalnon commented May 6, 2026

Summary

`PATCH /v1/datasets/batch-tags` is an `async def` route handler
that called `store.get_meta(...)` and `store.update_meta(...)`
synchronously inside a per-dataset loop. Those are filesystem I/O
calls; running them on the FastAPI event loop blocks every other
in-flight request for the duration of the batch.

Surfaced by the 2026-05-05 roadmap audit
(`notes/ROADMAP_AUDIT_2026-05-05.md` §4.1, BUG-JD-10).

Fix

Wrap each `store.get_meta` and `store.update_meta` call in
`await asyncio.to_thread(...)`. The async-wrap pattern is already
applied elsewhere in the same file (line 142, `generator_class.generate`);
this brings `batch_update_tags` in line.

User-visible behaviour unchanged — only event-loop fairness improves.

Test plan

  • New `test_store_calls_run_off_event_loop` — patches the
    memory_store fixture's `get_meta` / `update_meta` to record
    their executing thread, drives a 2-dataset batch through the
    route, and asserts every call ran on a worker thread (not the
    main thread). Future removal of the `asyncio.to_thread`
    wrapping fails the test with a clear "BUG-JD-10 regression"
    message.
  • Existing 6 cases in `TestBatchUpdateTags` still pass; total
    7 green.
  • Pre-commit hooks pass (ruff + bandit + mypy).

🤖 Generated with Claude Code

…G-JD-10)

``PATCH /v1/datasets/batch-tags`` is an ``async def`` route handler
that called ``store.get_meta(...)`` and ``store.update_meta(...)``
synchronously inside a per-dataset loop. Those are filesystem I/O
calls; running them on the FastAPI event loop blocks every other
in-flight request for the duration of the batch. The async-wrap
pattern (``await asyncio.to_thread(...)``) is applied elsewhere in
the same file (line 142, ``generator_class.generate``) but was
missed on this one route.

Surfaced by the 2026-05-05 roadmap audit
(``notes/ROADMAP_AUDIT_2026-05-05.md`` §4.1, BUG-JD-10).

Fix
---
Wrap each ``store.get_meta`` and ``store.update_meta`` call in
``await asyncio.to_thread(...)`` so the synchronous I/O runs on
the default executor while the event loop stays free to service
other requests. The user-visible behaviour is unchanged; only the
event-loop fairness improves.

Tests
-----
- New regression case ``test_store_calls_run_off_event_loop`` —
  patches both store methods to record their executing thread,
  drives a 2-dataset batch through the route, and asserts every
  call ran on a worker thread (not the main thread). Future
  removal of the ``asyncio.to_thread`` wrapping fails the test
  with a clear "BUG-JD-10 regression" message naming the
  expected wrapping.
- Existing 6 cases in ``TestBatchUpdateTags`` still pass; total
  7 cases green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@pcalnon pcalnon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

@pcalnon pcalnon merged commit aae0081 into main May 6, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant